Skip to content

ssh keys#2194

Merged
taylordolan merged 42 commits intomasterfrom
SAN-6011-SSH-keys
Apr 24, 2017
Merged

ssh keys#2194
taylordolan merged 42 commits intomasterfrom
SAN-6011-SSH-keys

Conversation

@taylordolan
Copy link
Copy Markdown
Member

@taylordolan taylordolan commented Apr 18, 2017

@taylordolan taylordolan requested a review from runnabro April 19, 2017 00:49
@taylordolan taylordolan removed the hold label Apr 19, 2017
use(
xlink:href = "#icons-octicons-github"
)
| Authorize…
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be the external link icon instead of ellipsis?

.grid-block.vertical.padding-left-sm.padding-right-sm
p.strong {{key.username}} user key (Runnable)
p.monospace.text-gray.text-overflow {{key.fingerprint}}
.btn.btn-xs.grid-block.shrink.align-center.btn-authorized
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would look better if it were the same width as the authorize button.

screen shot 2017-04-19 at 12 12 34 pm

ng-if = "listening"
) Listening for authorization…

a.btn.gray.btn-xs.grid-block.shrink.noscroll.align-center(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be its own template. We should pass in unauthorized, loading, and authorized states to show different icons/text.

@@ -0,0 +1,82 @@
section.label-description
.label-col SSH Keys
.small.text-gray All keys will be added to each container at build time. Keys can be deleted from 
Copy link
Copy Markdown
Member

@runnabro runnabro Apr 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe better worded as: "Keys are added to containers at build and run time, and are managed on GitHub."

) your SSH key settings
|  on GitHub.

ol.list-bordered.padding-left-sm.padding-right-sm(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iPhone SE

screen shot 2017-04-19 at 12 19 08 pm

header.modal-header
h1.modal-heading Authorized!
section.modal-body
.text-center.text-gray.padding-md You successfully authorized with GitHub 👍
Copy link
Copy Markdown
Member

@runnabro runnabro Apr 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing period? Also, can these classes be applied to .modal-body instead, to have one less div?

.grid-block.vertical.padding-left-sm.padding-right-sm(
ng-if = "!listening"
)
p.text-gray.strong Add your user key
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing period?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this list-item doesn't look enough like an empty state. It's hard to see that the avatar is half opacity.

.label-col SSH Keys
.small.text-gray All keys will be added to each container at build time. Keys can be deleted from 
a.link(
href = "#"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -0,0 +1,13 @@
.grid-block.align-center.justify-center.modal-backdrop.modal-ssh-auth
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

~iPhone size

screen shot 2017-04-21 at 10 48 24 am

border-color: $blue-light;
box-shadow: inset 0 0 0 $input-border-lg $blue-light;
height: 90px + $input-border * 2;
margin: -$input-border (-($sm + $input-border));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a fan of the negative margin-left/right

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also is the margin-bottom: -1px intentional?

ng-if = "state.creating"
ng-include = "'spinner'"
)
| {{creating ? 'Creating…' : 'Create Key'}}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

state.creating?

@@ -0,0 +1,70 @@
section.label-description
.label-col SSH Keys
.small.text-gray Keys will be added to containers at build and run time, and can be  
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be managed on, or are managed on?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer "can be". For some reason the tense of "are" throws me off.

//- li.grid-block.align-center.justify-center.list-item
.spinner-wrapper.spinner-sm.spinner-gray(
ng-include = "'spinner'"
)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to add a new loading state or use the existing default loading state for the forms?

Comment thread client/templates/svg/svgDefs.jade Outdated
rect(x='7', y='5', fill='#AD8850', width='1', height='3')
rect(x='10', y='10', fill='#AD8850', width='1', height='6')
path(d='M15.627,2.286A4.242,4.242,0,0,0,8.876,7.27L1.131,15.014,2.9,16.782l1.414-1.414L5.9,16.959l1.768-1.816L6.081,13.5l1.591,1.639,1.768-1.72L7.849,11.832l2.795-2.794a4.283,4.283,0,0,0,2,.5A4.2,4.2,0,0,0,15.627,8.3,4.257,4.257,0,0,0,15.627,2.286Zm-3,4.505a1.5,1.5,0,1,1,1.5-1.5A1.5,1.5,0,0,1,12.622,6.791Z', style='fill:#fec785;stroke:#cb9f6a')
symbol#icons-key-empty(viewBox='0 0 18 18')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indentation is off

@taylordolan taylordolan merged commit 6fffd34 into master Apr 24, 2017
@taylordolan taylordolan deleted the SAN-6011-SSH-keys branch April 24, 2017 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants